-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mark atomics as unsupported on thumbv6m #99595
Conversation
The thumbv6m target does not support atomics. Historically, LLVM had a bug where atomic load/stores for this target were emitted as plain load/stores rather than as libatomic calls. This was fixed in https://reviews.llvm.org/D120026, which will be part of LLVM 15. As we require that "atomic support" does not use libatomic, we need to indicate that this target does not have native atomics.
r? @nagisa (rust-highfive has picked a reviewer for you, use r? to override) |
|
To be precise, we require at atomics be lock-free. It is possible to have lock-free atomics backed by a library, such as when OS support is available (such as on This change is likely to break a lot of existing code using atomic load/store on thumbv6m (cc @japaric). I think this change makes sense for LLVM where you want all atomic operations to be either provided by a library or implemented natively. However Rust doesn't expose libatomic operations so this concern doesn't apply. I see 2 solutions here:
|
Right. Just to spell it out the relation explicitly: Lock-free atomic libcalls would use
If preserving non-CAS atomics for thumbv6m is important, we could probably add something similar to the existing |
18: Use asm-based atomic load/store on thumbv6m r=taiki-e a=taiki-e Atomic types on thumbv6m will be removed from the standard library: rust-lang/rust#99595 This patch implements the equivalent of the atomic types that will be removed in rust-lang/rust#99595. Note that this patch does *not* drop support for compilers older than 1.59. Co-authored-by: Taiki Endo <[email protected]>
Mark atomics as unsupported on thumbv6m The thumbv6m target does not support atomics. Historically, LLVM had a bug where atomic load/stores for this target were emitted as plain load/stores rather than as libatomic calls. This was fixed in https://reviews.llvm.org/D120026, which will be part of LLVM 15. As we require that "atomic support" does not use libatomic, we need to indicate that this target does not have native atomics.
…laumeGomez Rollup of 6 pull requests Successful merges: - rust-lang#99298 (Make `ui-fulldeps/gated-plugins` and `ui-fulldeps/multiple-plugins` tests stage 2 only) - rust-lang#99396 (Add some additional double-adjustment regression tests) - rust-lang#99449 (Do not resolve associated const when there is no provided value) - rust-lang#99595 (Mark atomics as unsupported on thumbv6m) - rust-lang#99627 (Lock stdout once when listing tests) - rust-lang#99638 (Remove Clean trait implementation for hir::Ty and middle::Ty) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
What are we supposed to do with all this existing code that uses AtomicUxx load/stores (e.g. on the Raspberry Pi RP2040)? Will it just no longer compile? |
Yes, they will fail to compile. (Can be checked by using this target specification file) |
This would revert #51953 which I think would be wrong. Afaik atomic load and store are available, but CAS is missing, which is covered by atomic_cas: false. |
Just adding another voice to this: if this change lands in stable without a mechanism to still use atomic load/stores (even if the underlying rust impl changes), this will be a huge stable to stable regression and breaking change for thumbv6m targets. |
I opened #99668 to track this. |
Thank you @Amanieu ! |
This reverts commit a864da7. rust-lang/rust#99595 has been reverted, so this is no longer needed.
The thumbv6m target does not support atomics. Historically, LLVM
had a bug where atomic load/stores for this target were emitted
as plain load/stores rather than as libatomic calls. This was
fixed in https://reviews.llvm.org/D120026, which will be part of
LLVM 15. As we require that "atomic support" does not use libatomic,
we need to indicate that this target does not have native atomics.